Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

factor out google analytics ids (LMS-2555) #3490

Closed
wants to merge 1 commit into from

Conversation

adampalay
Copy link
Contributor

@rocha
@feanil , this requires a change to our servers before we merge this

@symbolist
Copy link
Contributor

👍

@@ -397,3 +397,7 @@

##### ADVANCED_SECURITY_CONFIG #####
ADVANCED_SECURITY_CONFIG = ENV_TOKENS.get('ADVANCED_SECURITY_CONFIG', {})

##### GOOGLE ANALYTICS IDS #####
GOOGLE_ANALYTICS_MAIN = ENV_TOKENS.get('GOOGLE_ANALYTICS_MAIN')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to AUTH_TOKENS

@e0d
Copy link
Contributor

e0d commented Apr 28, 2014

Does this include updating from what is in the public domain or is that an additional step.

@adampalay
Copy link
Contributor Author

@e0d , what do you mean by that?

@e0d
Copy link
Contributor

e0d commented Apr 28, 2014

There are two token dicts, one loaded from, say, lms.env.json, one from lms.auth.json. That latter is used for keys, etc.

@adampalay
Copy link
Contributor Author

@e0d , yeah I changed that already--I was wondering what you meant about the public domain. Like cleaning up our git history?

@e0d
Copy link
Contributor

e0d commented Apr 28, 2014

No, meaning are the keys referenced in the Jira item new or the ones that are already in git history?

@e0d
Copy link
Contributor

e0d commented Apr 28, 2014

Also, do use GA for the the cms?

@adampalay
Copy link
Contributor Author

@e0d , the keys referenced in JIRA are the ones in the git history. Changing those would be an additional step.

As far as I can tell, no. google_analytics.html is only included in LMS templates, and grepping for "UA-\d+" elsewhere didn't return anything

@e0d
Copy link
Contributor

e0d commented Apr 28, 2014

👍

@rocha
Copy link
Contributor

rocha commented Apr 28, 2014

LGTM 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants